Codecov Report

Merging #508 into develop will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           develop    #508      +/-   ##
==========================================
+ Coverage    89.59%   89.6%   +0.01%     
==========================================
  Files           32      32              
  Lines         2815    2819       +4     
==========================================
+ Hits          2522    2526       +4     
  Misses         293     293

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c08dd0f...eef091a. Read the comment docs.

Thanks a lot for tackling this @maxcopeland. Please excuse the long wait time, but I was busy for the last few days finishing up a paper.

In addition to my comment, could you please:

• add a unit test
• from my understanding, a comma is legal in the nominal value (as long as it's properly quoted) (@janvanrijn please confirm). Therefore, I think you need to parse this a bit differently, for example using the csv module or ast.literal_eval.

@janvanrijn is there a reason that this is passed as a python-list-like string and not as a proper XML list? I assume the second one would be easier to handle by the client APIs as the parsing is handled by XML libraries.

@mfeurer no particular reason, if you prefer an xml list we can discuss it next week at the Hackathon?

I definitively prefer the list to be in XML, because we can then rely on standard libraries to do the parsing. If the list is stored like it is stored right now, this requires custom parsing on the client side, which we should definitively avoid.

No problem. Should I rewrite the parsing to expect xml list in xmlfeature['oml:nominal_values']?

Thanks for your offer. I'm afraid we first need to discuss how to expect such information from the server and then implement this. We will come back to you next week.

Okay, sounds good. Just let me know once there's a consensus and I'll make the necessary changes.

@janvanrijn just finished implementing getting the nominal values as an XML list. Could you please try again @maxcopeland ?

@mfeurer Great. Sure-- I'll get started.

Superseded by #644.